Skip to content

Conversation

@mfuntowicz
Copy link
Member

@mfuntowicz mfuntowicz commented Nov 28, 2020

  • Increased the tolerance when comparing Flax et PyTorch output (~0.00058 on my dev box)
  • Removed the jit parametrization when running test_multiple_sentences because it leads to instabilities
  • Introduced subtests expliciting what we're doing by enabling / disabling JIT.

Signed-off-by: Morgan Funtowicz <funtowiczmo@gmail.com>
Signed-off-by: Morgan Funtowicz <funtowiczmo@gmail.com>
Signed-off-by: Morgan Funtowicz <funtowiczmo@gmail.com>
…weight from the hub.

Signed-off-by: Morgan Funtowicz <funtowiczmo@gmail.com>
…cumenting.

Signed-off-by: Morgan Funtowicz <funtowiczmo@gmail.com>
Signed-off-by: Morgan Funtowicz <funtowiczmo@gmail.com>
Signed-off-by: Morgan Funtowicz <funtowiczmo@gmail.com>
Signed-off-by: Morgan Funtowicz <funtowiczmo@gmail.com>
Signed-off-by: Morgan Funtowicz <funtowiczmo@gmail.com>
Copy link
Contributor

@patrickvonplaten patrickvonplaten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it a lot! makes the tests easier to read and seems to fix CI - thanks a lot!

@mfuntowicz mfuntowicz requested a review from sgugger November 28, 2020 18:39
Copy link
Member

@LysandreJik LysandreJik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! Thanks for looking into it.

Signed-off-by: Morgan Funtowicz <funtowiczmo@gmail.com>
Copy link
Collaborator

@sgugger sgugger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing!

@sgugger sgugger merged commit 51b0713 into master Nov 30, 2020
@sgugger sgugger deleted the fix-flax-ci branch November 30, 2020 18:43
stas00 pushed a commit to stas00/transformers that referenced this pull request Dec 5, 2020
* Slightly increase tolerance between pytorch and flax output

Signed-off-by: Morgan Funtowicz <funtowiczmo@gmail.com>

* test_multiple_sentences doesn't require torch

Signed-off-by: Morgan Funtowicz <funtowiczmo@gmail.com>

* Simplify parameterization on "jit" to use boolean rather than str

Signed-off-by: Morgan Funtowicz <funtowiczmo@gmail.com>

* Use `require_torch` on `test_multiple_sentences` because we pull the weight from the hub.

Signed-off-by: Morgan Funtowicz <funtowiczmo@gmail.com>

* Rename "jit" parameter to "use_jit" for (hopefully) making it self-documenting.

Signed-off-by: Morgan Funtowicz <funtowiczmo@gmail.com>

* Remove pytest.mark.parametrize which seems to fail in some circumstances

Signed-off-by: Morgan Funtowicz <funtowiczmo@gmail.com>

* Fix unused imports.

Signed-off-by: Morgan Funtowicz <funtowiczmo@gmail.com>

* Fix style.

Signed-off-by: Morgan Funtowicz <funtowiczmo@gmail.com>

* Give default parameters values for traced model.

Signed-off-by: Morgan Funtowicz <funtowiczmo@gmail.com>

* Review comment: Change sentences to sequences

Signed-off-by: Morgan Funtowicz <funtowiczmo@gmail.com>

* Apply suggestions from code review

Co-authored-by: Sylvain Gugger <35901082+sgugger@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants